feat(sozo): scaffold ts base files on user behalf #2275
Conversation
WalkthroughOhayo, sensei! The recent changes introduce a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant BuildArgs
participant PluginManager
participant TypescriptScaffoldPlugin
participant Tera
User->>CLI: Initiates build command with --scaffold
CLI->>BuildArgs: Parses command with scaffold=true
BuildArgs->>PluginManager: Adds TypescriptScaffoldPlugin
PluginManager->>TypescriptScaffoldPlugin: Creates instance
TypescriptScaffoldPlugin->>Tera: Loads templates
TypescriptScaffoldPlugin->>User: Generates TypeScript files
Recent review detailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (1)
Files selected for processing (11)
Files skipped from review due to trivial changes (2)
Files skipped from review as they are similar to previous changes (4)
Additional context usedBiome
Additional comments not posted (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (7)
crates/dojo-bindgen/src/plugins/typescript_scaffold/tests.rs (2)
7-18: Consider verifying all expected keys in the test.The test
test_generate_code_contains_all_keyschecks if the result contains five keys, but it doesn't verify the specific keys expected. Consider asserting the presence of each key to ensure comprehensive testing.assert!(res.contains_key("setup.ts")); assert!(res.contains_key("world.ts")); assert!(res.contains_key("system.ts")); assert!(res.contains_key("clientModels.ts")); assert!(res.contains_key("clientComponents.ts"));
20-33: Ensure the test checks the content of the system file.The test
test_handle_systemcurrently retrieves the system file but does not verify its content. Consider adding assertions to check if the content matches expected patterns or values.assert!(system.contains("expected content or pattern"));crates/dojo-bindgen/src/plugins/typescript_scaffold/templates/setup.tpl.ts (2)
26-39: Consider improving error messages for clarity.The error messages in the
setupfunction could provide more context for easier debugging. Consider including more details about the failure context.- console.error("Failed to create torii client:", e); + console.error("Failed to create torii client with config:", config, "Error:", e);
116-117: Improve error message for active account retrieval.The error message when failing to get the active account could be more descriptive to aid in debugging.
- throw new Error("failed to get active account"); + throw new Error("Failed to retrieve active account from burner manager.");crates/dojo-bindgen/src/plugins/typescript_scaffold/mod.rs (2)
14-23: Consider using logging instead of println for errors.Using a logging library instead of
println!for error messages can provide more control over log levels and outputs.log::error!("Parsing error(s): {}", e);
47-56: Clarify the generated header comment.The header comment generated for files could be more informative. Consider adding more details about the purpose of the file or the context in which it was generated.
- // Generated by dojo-bindgen on {}. + // This file was generated by dojo-bindgen on {} as part of the TypeScript scaffolding process.bin/sozo/src/commands/build.rs (1)
31-33: Document thescaffoldargument clearly.The
scaffoldargument is added to enable Typescript scaffolding. Ensure the help documentation is clear and concise for users.- #[arg(help = "Generate Typescript scaffolding files.")] + #[arg(help = "Enable the generation of TypeScript scaffolding files.")]
| @@ -0,0 +1,5 @@ | |||
| {{ header }} | |||
There was a problem hiding this comment.
Remove the redundant block statement.
The block statement at the top of the file is unnecessary and can be safely removed to simplify the code.
-{{ header }}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{ header }} |
Tools
Biome
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
| @@ -0,0 +1,17 @@ | |||
| {{ header }} | |||
There was a problem hiding this comment.
Remove the redundant block statement.
The block statement at the top of the file is unnecessary and can be safely removed to simplify the code.
-{{ header }}Committable suggestion was skipped due to low confidence.
Tools
Biome
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
| @@ -0,0 +1,15 @@ | |||
| {{ header }} | |||
There was a problem hiding this comment.
Remove the redundant block statement.
The block statement at the top of the file is unnecessary and can be safely removed to simplify the code.
-{{ header }}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{ header }} |
Tools
Biome
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
| fn handle_file(&self, path: &str, mut ctx: tera::Context) -> String { | ||
| ctx.insert("header", &self.header().as_str()); | ||
| self.templates.render(path, &ctx).unwrap() | ||
| } |
There was a problem hiding this comment.
Handle template rendering errors gracefully.
Currently, unwrap() is used for template rendering, which can cause a panic if an error occurs. Consider handling this error more gracefully.
self.templates.render(path, &ctx).unwrap_or_else(|e| {
log::error!("Failed to render template {}: {}", path, e);
String::new()
})
MartianGreed
force-pushed
the
feat/ts-scaffold
branch
from
412585e
to
10592c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
crates/dojo-bindgen/src/plugins/typescript_scaffold/templates/setup.tpl.ts (2)
1-1: Remove redundant block statement.The
{ header }block statement doesn't serve any purpose and can be safely removed to simplify the code.- {{ header }}Tools
Biome
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
36-39: Enhance error logging for better debugging.Consider adding more context to error messages to improve debugging. For example, include the function or module name in the log.
- console.error("Failed to create torii client:", e); + console.error("setup: Failed to create torii client:", e); - console.error("Failed to create client:", e); + console.error("setup: Failed to create client:", e); - console.log("Failed to create burner manager:", e); + console.error("setup: Failed to create burner manager:", e);Also applies to: 76-78, 98-100
MartianGreed
force-pushed
the
feat/ts-scaffold
branch
from
10592c0
to
86e3aef
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
crates/dojo-bindgen/src/plugins/typescript_scaffold/templates/setup.tpl.ts (1)
1-1: Remove redundant block statement.Ohayo, sensei! The block statement
{{ header }}at the beginning of the file doesn't serve any purpose and can be safely removed to simplify the code.-{{ header }}Tools
Biome
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
(lint/complexity/noUselessLoneBlockStatements)
[error] 1-1: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
| {% for system in systems %} | ||
| const {{ system.name }} = async (account: AccountInterface) => { | ||
| throw new Error("Not implemented"); | ||
| }; | ||
|
|
||
| {% endfor %} | ||
| return { {% for system in systems %} {{ system.name }}, {% endfor %} }; | ||
| } |
There was a problem hiding this comment.
Correct template syntax for TypeScript compatibility.
Ohayo, sensei! The current template syntax is causing parsing errors. Ensure that the {% ... %} tags generate valid TypeScript code. Consider using template helpers or functions to encapsulate logic. This will help avoid syntax errors and improve maintainability.
If you need assistance with this, let me know!
Tools
Biome
[error] 27-27: Expected a statement but instead found '%'.
Expected a statement here.
(parse)
[error] 27-27: expected
(but instead foundsystemRemove system
(parse)
[error] 27-27: Expected an expression but instead found '}'.
Expected an expression here.
(parse)
[error] 28-28: Expected an identifier, a member name, or a rest pattern but instead found '{ system.name'.
Expected an identifier, a member name, or a rest pattern here.
(parse)
[error] 28-28: Object and Array patterns require initializers.
This pattern is declared, but it is not given an initialized value.
(parse)
[error] 28-28: Expected a statement but instead found '= async (account: AccountInterface) =>'.
Expected a statement here.
(parse)
[error] 32-32: Expected a statement but instead found '% endfor %'.
Expected a statement here.
(parse)
[error] 33-33: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '{% for system in systems %'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 33-33: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 33-33: Expected a statement but instead found ','.
Expected a statement here.
(parse)
[error] 33-33: Expected a statement but instead found '% endfor %'.
Expected a statement here.
(parse)
[error] 34-34: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
[error] 30-30: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[error] 33-33: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[error] 27-27: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
[error] 28-30: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
Description
As a developper when I start a new project I don't want to copy/paste or type by hand every dojo files required for the project to run.
I want to be able to have a command to scaffold dojo files in order to ease project starting and be able to choose the frontend architecture I want.
Related issue
Tests
Added to documentation?
I have documented the code itself + PR here on dojo.js
Checklist
scripts/prettier.sh,scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)Summary by CodeRabbit
New Features
TypescriptScaffoldPluginto facilitate TypeScript code generation.Bug Fixes
Tests
TypescriptScaffoldPluginto ensure correct functionality.